Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RespondWorkflowTask capabilities #467

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

Quinn-With-Two-Ns
Copy link
Contributor

Fix RespondWorkflowTask capabilities, I put it on the Response instead of the Request because I am dumb.

Note: This is technically a breaking change, but this was not released anywhere

@Quinn-With-Two-Ns Quinn-With-Two-Ns requested review from a team as code owners October 23, 2024 05:00
@alexshtin
Copy link
Member

Two reviewers approved original #458 PR for the same reason.

@Quinn-With-Two-Ns Quinn-With-Two-Ns merged commit efd54e9 into temporalio:master Oct 23, 2024
3 checks passed
alexshtin added a commit to temporalio/temporal that referenced this pull request Nov 18, 2024
## What changed?
<!-- Describe what has changed in this PR -->
Drop speculative WFT even if it had events.

## Why?
<!-- Tell your future self why have you made these changes -->
Special compatibility flag was
[added](temporalio/api#467) to
`RespondWorkflowTaskCompletedRequest`. If this flag is set to true, then
server can drop speculative WFT with update rejections, even if it
shipped events to the worker. SDK support will be added later.

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Added new unit tests.

## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
No risks. Compatibility flag is used to support backward compatibility
and don't drop speculative WFT if worker doesn't support it (old go
SDKs).

## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->
Yes, updated.

## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
No.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants